Redesign fork choice abstractions#5249
Merged
Merged
Conversation
jtraglia
reviewed
May 13, 2026
jtraglia
reviewed
May 13, 2026
jtraglia
approved these changes
May 19, 2026
jtraglia
left a comment
Member
There was a problem hiding this comment.
I like these changes! Most of my comments here are little nits.
Member
|
@mkalinin just waiting for @jihoonsong to finish his review of this. I've been chatting with him privately and he's expecting to finish tomorrow. After that feedback is resolved, I'm confident we will merge this PR 🙂 |
brech1
reviewed
May 22, 2026
* Move get_block_root_node to FCR * Polish naming * Fix lint
jihoonsong
reviewed
May 22, 2026
jihoonsong
reviewed
May 22, 2026
jihoonsong
reviewed
May 27, 2026
jihoonsong
left a comment
Member
There was a problem hiding this comment.
Looks good. I think it's good to merge once the nits are fixed. I will make a following PR to handle the note comments across forks, so we can accept them as-is IMO. I don't want to block this PR just for them.
Co-authored-by: Jihoon Song <jihoonsong@users.noreply.github.com>
jihoonsong
approved these changes
May 27, 2026
jihoonsong
left a comment
Member
There was a problem hiding this comment.
Great work, thank you for cleaning fork choice spec! Looking forward to have the next PR on revamping get_weight() but no rush :)
brech1
approved these changes
May 27, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This proposal makes
ForkChoiceNodea first class citizen since Phase0 and extends it in Gloas.Motivation
get_headreturns aForkChoiceNoderegadless of a fork, there is no need tocheck with the spec version when designing fork choice tests that are common for all versions
over plain beacon block roots. A good example of it would be Fast Confirmation Rule spec.
Changes
There are no semantics changes in Phase0 and Gloas. However, there are minor changes in some Gloas function behavior.
We will go over each meaningful change below.
get_block_root_nodeIntroduced in Phase0 and modified in Gloas. This is a synthetic function that creates aForkChoiceNodewith ablock_rootthat is a common ancestor of all nodes with the sameblock_root. This function is handy for fast confirmation rule and potentially other spec code relying on the fork choice but operating over a beacon block tree only which implies that that code is agnostic to other fork choice node attributes like payload status.Alternative could be a default value forForkChoiceNode.payload_statusfield in Gloas. But I preferred this abstraction to be more explicit.The naming of this function can be improved.UPD: Replaced with
get_node_for_rootintroduced for the FCR only andget_fork_choice_nodetest helper.get_ancestorAccepts
node: ForkChoiceNodeas a parameter instead ofroot: Root.The difference between existing Gloas version and this PR is as the following. When
block.slot <= slotthe existing version always returns a node withPAYLOAD_STATUS_PENDINGpayload status androot = block_root. The new version returnsnodeas is in this case. Whenblock.slot > slotthe results of existing and new versions are equivalent.Behavior of this function in a new Gloas version is equivalent to Phase0 except for the way it obtains a parent node. In gloas parent node is always created with payload status returned by
get_parent_payload_status.is_ancestorGets ancestor of a
nodeatmaybe_ancestor.slotand checks if they are equal.In Gloas the above equality check is preserved and, if
ancestor.root == maybe_ancestor.root, payload statuses ofnodeandmaybe_ancestorare checked. ReturnsTrueif payload statuses are equal ormaybe_ancestorpayload status isPAYLOAD_STATUS_PENDING.get_checkpoint_blockModified to use
ForkChoiceNode.get_supported_nodeIn Phase0 returns
ForkChoiceNode(root=message.root).In Gloas modified to return
ForkChoiceNode(root=message.root, payload_status=payload_status), wherepayload_statusis borrowed frommessage.payload_presentin case whenblock.slot < message.slotand equal toPAYLOAD_STATUS_PENDINGotherwise.get_attestation_scoreModified to use
is_ancestor(store, get_supported_node(store, store.latest_messages[i]), node)in Phase0 and Gloas.In Phase 0
is_ancestorcall result equals toget_ancestor(store, store.latest_messages[i].root, store.blocks[root].slot) == root.In Gloas it has been handled by
is_supporting_votebefore which is deprecated by this PR. Now to the equivalence between existing and new versions:Case 1.
node.root == message.root. Note that in this caseget_ancestor(store, supported_node, store.blocks[node.root].slot)that is called inside ofis_ancestorwill returnancestor == supported_nodeasstore.blocks[supported_node.root].slot == store.blocks[node.root].slot). Note thatmaybe_ancestorin the new version equals tonodein the existing version. With this in mind we proceed with the following table:node.payload_status == PAYLOAD_STATUS_PENDINGTruenode.payload_status == PAYLOAD_STATUS_PENDINGwhich isTruenode.payload_status != PAYLOAD_STATUS_PENDINGandmessage.slot == block.slotFalsesupported_node.payload_statusis alwaysPAYLOAD_STATUS_PENDINGas theblockit supports is from the same slot as the message (as perget_supported_node), thussupported_node.payload_status != node.payload_statusandnode.payload_status != PAYLOAD_STATUS_PENDINGwhich implies the result isFalsenode.payload_status != PAYLOAD_STATUS_PENDINGandmessage.slot > block.slotnode.payload_status == (PAYLOAD_STATUS_FULL if message.payload_present else PAYLOAD_STATUS_EMPTY)supported_node.payload_status == node.payload_statusis returned asnode.payload_status == PAYLOAD_STATUS_PENDINGisFalse,supported_node.payload_status = PAYLOAD_STATUS_FULL if message.payload_present else PAYLOAD_STATUS_EMPTYas perget_supported_node, thus the results are equivalentCase 2.
node.root != message.root.If
node.rootandmessage.rootare from the same slot then both versions returnFalsebecause of arootmismatch.Otherwise,
node.rootmust be from a past slot (no attestations from the future are applied to the FC). Thenancestor = get_ancestor(store, message.root, block.slot)in the original version gives the same result asancestor = get_ancestor(store, supported_node, store.blocks[node.root].slot)in the new version. And then exactlynode.payload_status == PAYLOAD_STATUS_PENDING or node.payload_status == ancestor.payload_statusis returned in both cases.get_weightInstead of relying on
is_supporting_votefor proposer boost it relies onis_ancestor(store, proposer_boost_node, node).proposer_boost_nodein Gloas is constructed withPAYLOAD_STATUS_PENDING. In Phase0 it is simply aForkChoiceNode(root=store.proposer_boost_root).get_node_childrenThis function is introduece to Phase0 and then overridden in Gloas.
get_headReturns
ForkChoiceNodein Phase0 and the new version. New version code is very similar to Phase0 except for initial node construction and introduction ofget_payload_status_tiebreakerTesting
get_head(store)and other fork choice functions are usedis_ancestorhelper is introduced that works with both types, beacon block roots and fork choice nodes, and handles that inside of the functionmake reftests=true testpasses